Conversation
It's a bit rough and some of the timing isn't perfect, but: https://github.com/user-attachments/assets/bfee2fa5-b4cd-4665-b70f-d902ad93ae43 Also adds some silly placeholder sounds to the end screen: https://github.com/user-attachments/assets/31e34067-4343-4464-9777-f93c8cad1021 --- - [x] depends on ppy/osu-resources#415 --------- Co-authored-by: Dean Herbert <pe@ppy.sh>
This has gone through a few iterations, and eventually ended up as a simple text percentage display next to the username. I feel that adding another progress bar right next to the big healthbar would make things too cluttered, and trying to move the beatmap state elsewhere would make it too disconnected from the players that are potentially downloading a beatmap. I considered making the local user fetch download progress data using `BeatmapDownloadTracker` instead of relying on `BeatmapAvailability` in order to get more frequent updates, but that would add a lot of extra complexity for little gain IMO. [Screencast_20260403_095644.webm](https://github.com/user-attachments/assets/85fbd4b8-6b5c-41d2-b29b-c93885f73bb3)
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis pull request adds audio support and beatmap availability UI to the ranked play matchmaking system. Changes include test updates for validating beatmap state transitions, UI enhancements to display beatmap availability with live multiplayer updates, audio sample playback during ranked match endings and results screens, and a resource dependency version update. Changes
Sequence Diagram(s)sequenceDiagram
participant RC as ResultScreenContent<br/>(Load Phase)
participant AM as AudioManager
participant SC as SampleChannels<br/>(Playback)
RC->>AM: Preload result samples<br/>(appear, damage, HP, rank)
AM-->>RC: Sample instances
rect rgba(100, 150, 200, 0.5)
Note over RC,SC: Appear Sequence
RC->>SC: Play results-appear sample
SC-->>RC: Audio playing
end
rect rgba(100, 200, 150, 0.5)
Note over RC,SC: Score/Counter Animation
RC->>AM: Create score-tick channels
AM-->>SC: Stereo-balanced channels
RC->>SC: Schedule counter playback
SC-->>RC: Audio loops/updates
end
rect rgba(200, 150, 100, 0.5)
Note over RC,SC: Damage/Health Updates
RC->>RC: Compute damage source
RC->>SC: Play damage-fly sample<br/>(stereo-panned)
RC->>SC: Play damage-hit sample
SC-->>RC: Damage audio sequence
end
rect rgba(150, 100, 200, 0.5)
Note over RC,SC: Rank Details Display
RC->>AM: Map rank to grade sample
RC->>SC: Play rank-impact audio
SC-->>RC: Rank audio complete
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Updates ranked play UI to better communicate match state through new SFX cues and beatmap availability indicators, alongside a resource package bump.
Changes:
- Bump
ppy.osu.Game.Resourcespackage version. - Add multiple SFX triggers to ranked play ended/results screens.
- Extend
RankedPlayUserDisplayto show beatmap download/import state and add/adjust visual tests.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| osu.Game/osu.Game.csproj | Updates resources package version required for new assets. |
| osu.Game/Screens/OnlinePlay/Matchmaking/RankedPlay/ResultsScreen.cs | Adds result/score/grade SFX and looping tick channels during counter animations. |
| osu.Game/Screens/OnlinePlay/Matchmaking/RankedPlay/EndedScreen.cs | Adds win/lose/draw SFX on match end screen. |
| osu.Game/Screens/OnlinePlay/Matchmaking/RankedPlay/Components/RankedPlayUserDisplay.cs | Displays beatmap availability state based on room updates and manages event subscription lifecycle. |
| osu.Game.Tests/Visual/RankedPlay/TestSceneRankedPlayUserDisplay.cs | Updates setup to join ranked room, adds beatmap availability state test, and adjusts user IDs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Schedule(() => | ||
| { | ||
| if (playerScoreTickChannel != null && playerScoreBar.Height >= playerScorePercent) | ||
| playerScoreTickChannel.Looping = false; | ||
|
|
||
| if (opponentScoreTickChannel != null && opponentScoreBar.Height >= opponentScorePercent) | ||
| opponentScoreTickChannel.Looping = false; | ||
| }); |
There was a problem hiding this comment.
Stopping the looping tick based on playerScoreBar.Height >= playerScorePercent (and same for opponent) is incorrect because Height is a lerped value in the range [0.05..1], not the underlying progress/percent; this can cause the loop to stop early (e.g., small percents). Compare against the actual progress value (e.g., e.NewValue >= playerScorePercent) instead. Also, scheduling a new action on every value change during transforms can enqueue many scheduled actions per frame; perform the stop check directly in the handler (or gate it so it runs once) to avoid unnecessary scheduler churn.
| Schedule(() => | |
| { | |
| if (playerScoreTickChannel != null && playerScoreBar.Height >= playerScorePercent) | |
| playerScoreTickChannel.Looping = false; | |
| if (opponentScoreTickChannel != null && opponentScoreBar.Height >= opponentScorePercent) | |
| opponentScoreTickChannel.Looping = false; | |
| }); | |
| if (playerScoreTickChannel != null && e.NewValue >= playerScorePercent) | |
| playerScoreTickChannel.Looping = false; | |
| if (opponentScoreTickChannel != null && e.NewValue >= opponentScorePercent) | |
| opponentScoreTickChannel.Looping = false; |
| cornerPiece?.OnHealthChanged(e.NewValue); | ||
| }); | ||
|
|
||
| client.RoomUpdated += onRoomUpdated; |
There was a problem hiding this comment.
The beatmap availability text won’t update until the next RoomUpdated event fires. If the room already has a non-default availability when this drawable appears, beatmapState will show the wrong initial state. Call onRoomUpdated() once after subscribing (or immediately before subscribing) so the UI reflects the current room state on first display.
| client.RoomUpdated += onRoomUpdated; | |
| client.RoomUpdated += onRoomUpdated; | |
| onRoomUpdated(); |
| @@ -36,23 +46,38 @@ public override void SetUpSteps() | |||
| [Test] | |||
| public void TesUserDisplay() | |||
There was a problem hiding this comment.
Test method name has a typo (TesUserDisplay). Rename to TestUserDisplay for consistency with other tests and to improve discoverability in test runners and logs.
| public void TesUserDisplay() | |
| public void TestUserDisplay() |
Summary by Gitar
RankedPlayUserDisplaycomponentRankedPlayUserDisplayppy/osu#37188This will update automatically on new commits.
Summary by CodeRabbit
New Features
Chores